Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map - commented code for costing allowing other types of variable in the TimeSlider #1075

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bobular
Copy link
Member

@bobular bobular commented May 7, 2024

Works towards #761

  • comments relating to allowing other continuous variables (number and integer)
  • comments relating to allowing string ordinals and decide on approach to take

Allowing the other continuous variables doesn't look too tricky.

It's looking like string ordinals (for example with 'YYYY-MM' values) may require a separate TimeSlider component for the following reasons

  • ordinals are filtered by checkboxes so despite being shown in date order, they could have gaps (e.g. no December)
  • therefore range selection and (in future) stepping forwards and backwards are going to be tricky
    • for example, the onChange callback of Brush will return a list of selected values (in domain.values) instead of the range (domain.x0 to domain.x1)
  • I'd (@bobular) suggest disabling all kinds of selection in the ordinal component (e.g. no Brush) - at least as a Phase 1.
    • later on it might be decided that a different UI is needed (e.g. discontinuous selections similar to checkboxes)
    • we will need appropriate user feedback when users try to make a selection to explain why they can't

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant